Skip to content

Output ES module from rollup#224

Merged
tannerlinsley merged 1 commit intoTanStack:masterfrom
warlo:master
Mar 10, 2020
Merged

Output ES module from rollup#224
tannerlinsley merged 1 commit intoTanStack:masterfrom
warlo:master

Conversation

@warlo
Copy link
Copy Markdown
Contributor

@warlo warlo commented Mar 10, 2020

After v1.0.12 our rollup build broke due to namedExports no longer being available to rollup due to the NODE_ENV + require() dev/prod logic. That forces us to explicitly alias all react-query exports in our rollup-config, however building an es module makes rollup happy.

I guess the dev / prod build is not that required for es modules, and libraries like redux only provides a single minified es build.

@tannerlinsley
Copy link
Copy Markdown
Member

Hmm. I thought the way it is currently set up was sufficient as we simply mimicked the way React itself was set up. If I recall correctly, React doesn't have an es module build does it?

@warlo
Copy link
Copy Markdown
Contributor Author

warlo commented Mar 10, 2020

You are probably right about react, and it might be a good reason for it not having one yet. (We actually use preact that provides an es module build). I see now that you had multiple outputs, including an es module, thats probably why it worked in our build prior to v1.0.12. However when you removed the module and made the index.js entrypoint the only option, rollup (or their commonjs plugin) is no longer able to resolve what is exported.
This does atleast give me:
[!] Error: 'useMutation' is not exported by node_modules/react-query/index.js
or even using the production build directly
[!] Error: 'useMutation' is not exported by node_modules/react-query/dist/react-query.production.min.js

It might be some bad config on our end, but regardless of that I am not sure if there is a reason not to provide an es module as an alternative. UMD is not statically analyzable afaik and generally used as fallback builds(?), which makes it harder for bundlers to leverage properties like tree-shaking etc.

@tannerlinsley
Copy link
Copy Markdown
Member

Is there any reason to use react-query.mjs over just react-query.es.js. Also, I don't think it should be minified, since that will likely happen with their own minifier at a later stage.

@warlo
Copy link
Copy Markdown
Contributor Author

warlo commented Mar 10, 2020

Node went with .mjs, and mozilla recommends and refers to node as well, so I guess it is still up for discussion. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules#Aside_%E2%80%94_.mjs_versus_.js

Personally not important with minification and as you say it will probably be minified anyways, but would it pose any issue? Sourcemaps should be available, and it might be preferable for users loading the module from a CDN or similar.

@tannerlinsley
Copy link
Copy Markdown
Member

Okay. In that case, we can keep it mjs. Okay, CDN usage makes sense. We can minify it until someone complains (maybe never)

@tannerlinsley tannerlinsley merged commit 5f881a9 into TanStack:master Mar 10, 2020
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Dec 3, 2024

View your CI Pipeline Execution ↗ for commit 7bcb5cc

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded <1s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2026-04-21 15:58:11 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants